-
Notifications
You must be signed in to change notification settings - Fork 17
MM-67108: Auto-generate Encryption Key on upgrade if missing #218
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
When upgrading to v1.7.0+, users would see an error if the Encryption Key was not set. This change auto-generates a valid 32-character key if it's missing or invalid, and saves it to the plugin config. - Added generateRandomKey() helper function - Added savePluginConfig() to persist auto-generated keys - Existing valid keys are not overwritten - Plugin now starts successfully on upgrade without manual intervention
jwilander
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. I would like to see us starting to add unit tests to integrations but I won't block the PR on it as there's hardly any unit tests in the plugin at all.
- Test generateRandomKey() generates correct length and unique keys - Test OnConfigurationChange auto-generates key when missing - Test existing valid keys are not overwritten
|
@jwilander Opus helped generate the test quite quickly! Ran them and they look good, and I can't think of a missing case, let me know if you want to see another example? |
|
Great! The only other case I can think of is a negative integer being passed to |
|
@jwilander thanks for the suggestion! I considered adding validation for negative integers but generateRandomKey is a private function only called internally with a hardcoded value of 32 i don't see a code path where a negative or zero value could be passed. |
ogi-m
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Verified the key gets auto-generated on upgrade
Summary
When upgrading to v1.7.0 users would see an error if the Encryption Key was not set. This change auto generates a valid 32-character key if it's missing or invalid and saves it to the plugin config.
QA steps
Expected: Plugin activates successfully without errors
Check System Console → Plugins → Confluence - Encryption Key should now be auto-populated
Ticket Link
https://mattermost.atlassian.net/browse/MM-67108